🐛 fix(registration): guard against race where addon config not yet synced#382
Conversation
Add ConfigCheckEnabled + Configured condition check to registration controller to prevent setting Status.Namespace before addon config is ready. When ConfigCheckEnabled is true, registration controller now waits for the Configured condition (set by OCM's addonconfiguration controller) before proceeding with registration. This prevents a race where the namespace from AddOnDeploymentConfig hasn't been synced to ManagedClusterAddOn status yet. The Configured condition and Status.ConfigReferences are set atomically by OCM's addonconfiguration controller, so Configured=True guarantees that ConfigReferences is populated. Re-reconcile is guaranteed by the informer pattern - when OCM sets Configured=True, the status update triggers an event and re-enqueues the addon. Updated both helloworld examples to use WithConfigCheckEnabledOption() to enable the full protection chain: - Registration controller: Configured check (new) - Deploy/hook syncers: ConfigCheckEnabled guard (existing) - Health checks: ManifestApplied guard (existing) Added documentation comments in health check code explaining the indirect protection via ManifestApplied condition dependency. Added test cases for: - ConfigCheckEnabled=true + Configured=False -> skips registration - ConfigCheckEnabled=true + Configured=True -> proceeds normally - ConfigCheckEnabled=false -> proceeds normally (backward compat) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Tesshu Flower <tflower@redhat.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tesshuflower The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
1 participant
Add ConfigCheckEnabled + Configured condition check to registration controller to prevent setting Status.Namespace before addon config is ready.
When ConfigCheckEnabled is true, registration controller now waits for the Configured condition (set by OCM's addonconfiguration controller) before proceeding with registration. This prevents a race where the namespace from AddOnDeploymentConfig hasn't been synced to ManagedClusterAddOn status yet.
The Configured condition and Status.ConfigReferences are set atomically by OCM's addonconfiguration controller, so Configured=True guarantees that ConfigReferences is populated.
Re-reconcile is guaranteed by the informer pattern - when OCM sets Configured=True, the status update triggers an event and re-enqueues the addon.
Updated both helloworld examples to use WithConfigCheckEnabledOption() to enable the full protection chain:
Added documentation comments in health check code explaining the indirect protection via ManifestApplied condition dependency.
Added test cases for:
Summary
This is currently a draft to investigate an alternative fix for the race condition.
The downside is this only protects when users enable the
ConfigCheckEnabledAgentAddOnOption.Related issue(s)
Fixes #open-cluster-management-io/ocm#1465